App Search: Result Component Updates #96184

daveyholler · 2021-04-03T00:05:38Z

Summary

This updates the result component to do the following:

Standardize the placement of document details
Remove the click target from the entire wrapper and instead place it (optionally) on the document ID and a dedicated view button
Horizontally align document actions (view, delete, curate) instead of vertically along the side
Adds an EuiToken next to fields that specify a field_type for easier visual grepping

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

daveyholler · 2021-04-03T00:07:07Z

@JasonStoltz I've made some pretty good headway here, but I'm gonna need a hand updating tests and such. I'm hoping you'd be willing to help take this over the finish line. I'm gonna throw some time on your calendar next week to sync about it 😀

JasonStoltz · 2021-04-07T17:18:24Z

x-pack/plugins/enterprise_search/public/applications/app_search/components/result/result.tsx

+              <EuiButtonIcon
+                iconType={iconType}
+                onClick={onClick}
+                color={iconColor ? 'primary' : undefined}


Ran across this while writing tests ... I assume this is a mistake and should be:

Suggested change

color={iconColor ? 'primary' : undefined}

color={iconColor ? iconColor : 'primary'}

Can you confirm?

Ha. Yes. iconColor : 'primary' 🤦🏼

JasonStoltz · 2021-04-07T18:27:32Z

@daveyholler I pushed updates for tests and types. Should be green now, just need to review 😫

JasonStoltz

I'm generally good with all this, nice work! @constancecchen Did you want to take a look before we merge it?

JasonStoltz · 2021-04-07T18:32:37Z

x-pack/plugins/enterprise_search/public/applications/app_search/components/result/result.tsx

-            ))}
-        </>
-      )}
+      <article className="appSearchResult__content">


Nice, I'm glad this entire section is no longer linked

JasonStoltz · 2021-04-07T18:39:25Z

FWIW, there is an axe failure on the drag handle. Not sure it's new though:

Issue Description
Ensures every ARIA button, link and menuitem has an accessible name

Element location
div[data-rbd-drag-handle-draggable-id="draggable-2"]

cee-chen · 2021-04-07T18:56:46Z

Ya it's not new, I noticed that failure as well when I implemented the original drag handle. It's out of scope for us IMO, something to be addressed by either EUI or the react-beautiful-dnd library.

I would like to take a quick review of this PR if that's cool! 👍

cee-chen · 2021-04-07T19:48:36Z

x-pack/plugins/enterprise_search/public/applications/app_search/components/result/result.scss

@@ -44,9 +45,13 @@
    display: flex;
    justify-content: center;
    align-items: center;
-    width: $euiSizeL * 2;
+    width: $euiSize * 2;


Suggested change

width: $euiSize * 2;

width: $euiSizeXL;

cee-chen · 2021-04-07T19:49:54Z

x-pack/plugins/enterprise_search/public/applications/app_search/components/result/result.scss

-
-/**
- * CSS for hover specific logic
- * It's mildly horrific, so I pulled it out to its own section here


💯 I'm super glad I pulled this out into its own easily delete-able section. H E L L Y E A

cee-chen · 2021-04-07T19:52:43Z

@elasticmachine merge upstream

daveyholler · 2021-04-07T20:05:48Z

I'ma take this out of Draft status and mark it Ready for Review.

JasonStoltz · 2021-04-08T13:34:26Z

Strange, I wasn't getting all of these type failures locally. Looking into them now.

cee-chen · 2021-04-08T15:09:21Z

@elasticmachine merge upstream

daveyholler · 2021-04-08T20:48:49Z

@elasticmachine merge upstream

kibanamachine · 2021-04-08T22:50:56Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`enterpriseSearch`	1322	1323	+1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`enterpriseSearch`	2.0MB	2.0MB	+793.0B

History

💔 Build #118269 failed f687a1b
💚 Build #118233 succeeded 8bbf023
💔 Build #118028 failed d3fbe43
💔 Build #117995 failed b2af29b
💔 Build #117142 failed 31cc2aa

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

* Starting work on result component * Write tests * Fix types * Cleanup * Fix type errors Co-authored-by: Jason Stoltzfus <[email protected]> Co-authored-by: Kibana Machine <[email protected]>

kibanamachine · 2021-04-08T22:54:15Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

* Starting work on result component * Write tests * Fix types * Cleanup * Fix type errors Co-authored-by: Jason Stoltzfus <[email protected]> Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Davey Holler <[email protected]> Co-authored-by: Jason Stoltzfus <[email protected]>

…to-metrics-tab * 'master' of github.com:elastic/kibana: (44 commits) [Exploratory View]Additional metrics for kpi over time (elastic#96532) [Fleet] UI changes on hosted policy detail view (elastic#96337) Stacked line charts incorrectly shows one term as 100% (elastic#96203) [Fleet] Create enrollment API keys as current user (elastic#96464) [Lens] Make table and metric show on top Chart switcher (elastic#96601) skip flaky suite (elastic#96691) [Lens] Hide "Show more errors" once expanded (elastic#96605) [Discover] Unskip histogram hiding test (elastic#95759) skip flyout test, add linked issue elastic#96708 skip copy_to_space_flyout_internal.test.tsx elastic#96708 fix config validation (elastic#96502) Document telemetry fields for stack security features (elastic#96638) [Partial Results] Move inspector adapter integration into search source (elastic#96241) [RAC] Rule registry plugin (elastic#95903) [APM] Run precommit tasks sequentially (elastic#96551) [Maps] fix Kibana does not recognize a valid geo_shape index when attempting to create a Tracking Containment alert (elastic#96633) [Security Solution] [Cases] Small UI bugfixes (elastic#96511) [Actions UI] Changed PagerDuty action form UI to fill payload fields according to the API docs for Resolve and Acknowledge events. (elastic#96363) App Search: Result Component Updates (elastic#96184) [Alerting] Preconfigured alert history index connector (elastic#94909) ...

Starting work on result component

31cc2aa

daveyholler requested a review from JasonStoltz April 3, 2021 00:05

JasonStoltz reviewed Apr 7, 2021

View reviewed changes

JasonStoltz added 2 commits April 7, 2021 13:55

Write tests

97cbb6f

Fix types

cd5488e

Cleanup

b2af29b

JasonStoltz approved these changes Apr 7, 2021

View reviewed changes

cee-chen reviewed Apr 7, 2021

View reviewed changes

Merge branch 'master' into as-document-component

d3fbe43

daveyholler marked this pull request as ready for review April 7, 2021 20:05

daveyholler requested a review from a team April 7, 2021 20:05

daveyholler added auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes v7.13.0 labels Apr 7, 2021

Fix type errors

8bbf023

Merge branch 'master' into as-document-component

f687a1b

Merge branch 'master' into as-document-component

8f8ed5c

daveyholler merged commit 0bf57a2 into elastic:master Apr 8, 2021

kibanamachine mentioned this pull request Apr 8, 2021

[7.x] App Search: Result Component Updates (#96184) #96674

Merged

cee-chen mentioned this pull request Apr 9, 2021

[App Search] Results follow-up #96709

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

App Search: Result Component Updates #96184

App Search: Result Component Updates #96184

daveyholler commented Apr 3, 2021

daveyholler commented Apr 3, 2021

JasonStoltz Apr 7, 2021

daveyholler Apr 7, 2021

JasonStoltz commented Apr 7, 2021

JasonStoltz left a comment

JasonStoltz Apr 7, 2021

JasonStoltz commented Apr 7, 2021

cee-chen commented Apr 7, 2021

cee-chen Apr 7, 2021

cee-chen Apr 7, 2021

cee-chen commented Apr 7, 2021

daveyholler commented Apr 7, 2021

JasonStoltz commented Apr 8, 2021

cee-chen commented Apr 8, 2021

daveyholler commented Apr 8, 2021

kibanamachine commented Apr 8, 2021

kibanamachine commented Apr 8, 2021

	color={iconColor ? 'primary' : undefined}
	color={iconColor ? iconColor : 'primary'}

App Search: Result Component Updates #96184

App Search: Result Component Updates #96184

Conversation

daveyholler commented Apr 3, 2021

Summary

Checklist

For maintainers

daveyholler commented Apr 3, 2021

JasonStoltz Apr 7, 2021

Choose a reason for hiding this comment

daveyholler Apr 7, 2021

Choose a reason for hiding this comment

JasonStoltz commented Apr 7, 2021

JasonStoltz left a comment

Choose a reason for hiding this comment

JasonStoltz Apr 7, 2021

Choose a reason for hiding this comment

JasonStoltz commented Apr 7, 2021

cee-chen commented Apr 7, 2021

cee-chen Apr 7, 2021

Choose a reason for hiding this comment

cee-chen Apr 7, 2021

Choose a reason for hiding this comment

cee-chen commented Apr 7, 2021

daveyholler commented Apr 7, 2021

JasonStoltz commented Apr 8, 2021

cee-chen commented Apr 8, 2021

daveyholler commented Apr 8, 2021

kibanamachine commented Apr 8, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

kibanamachine commented Apr 8, 2021

💚 Backport successful